Skip to content

feat: implement eth2util/hash#307

Closed
PoulavBhowmick03 wants to merge 1 commit intomainfrom
poulav/eth2util_hash
Closed

feat: implement eth2util/hash#307
PoulavBhowmick03 wants to merge 1 commit intomainfrom
poulav/eth2util_hash

Conversation

@PoulavBhowmick03
Copy link
Copy Markdown
Collaborator

Closes #303

Comment on lines +16 to +24
pub fn slot_hash_root(slot: Slot) -> Result<Root> {
let mut hasher = Hasher::default();
let index = hasher.index();

hasher.put_uint64(slot)?;
hasher.merkleize(index)?;

Ok(hasher.hash_root()?)
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need this helper, tree_hash::TreeHash should implement the tree_hash_root for Slot which is u64 already.

use tree_hash::TreeHash;

let tree_hash_root = slot.tree_hash_root().0;

@emlautarom1
Copy link
Copy Markdown
Collaborator

After further analysis seems like the PR is mostly boilerplate that we don't need.

For example, signeddata requires this module:

https://github.com/ObolNetwork/charon/blob/749d2d7ab0b8ace34f2e686a5af5910c8adb4dc0/core/signeddata.go#L1253-L1255

In Pluto, we don't really need the indirection of this module and we already sorted it out:

fn message_root(&self) -> Result<[u8; 32], Self::Error> {
Ok(hash_root(&self.0.slot))
}

The test pass as expected so the code they wrote manually is the same as the one we get from tree-hash as pointed by @iamquang95. I'm closing this PR and marking the issue as solved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement eth2util/hash

3 participants